[jules] enhance: Add animated success celebration #275
Conversation
- Implemented `useConfetti` hook using `canvas-confetti` - Added "Celebrate!" button to the "All Settled Up" state in GroupDetails - Trigger fireworks on successful payment recording - Supports both Glassmorphism and Neobrutalism themes naturally via the existing UI components Co-authored-by: Devasy23 <110348311+Devasy23@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
✅ Deploy Preview for split-but-wiser ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughAdds a Web Success Celebration feature: a new Changes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/pages/GroupDetails.tsx (1)
446-459: 🧹 Nitpick | 🔵 TrivialFireworks fire on every payment, not just the one that fully settles the group.
The PR objective says "Fireworks trigger automatically when recording a payment," which this implements. However, a 5-second fireworks animation on every payment recording (even a partial $5 payment in a group with $500 of outstanding debt) may feel disproportionate. Consider gating the fireworks on whether the payment actually settles all balances (i.e.,
settlements.length === 0afterfetchDatacompletes).If this is intentional for user delight on every payment, the current behavior is acceptable — just flagging the UX consideration.
Possible conditional fireworks approach
try { await createSettlement(id, { payer_id: paymentPayerId, payee_id: paymentPayeeId, amount: numAmount }); setIsPaymentModalOpen(false); setPaymentAmount(''); - fetchData(); + await fetchData(); addToast('Payment recorded successfully!', 'success'); - fireFireworks(); + // Fire fireworks only if all debts are now settled + // settlements state will be updated by fetchData() + // Alternatively, fire confetti for partial payments + if (settlements.length === 0) { + fireFireworks(); + } else { + fireConfetti(); + } } catch (err) {Note: This requires
fetchDatato be awaited so thesettlementsstate is updated before the check.
🤖 Fix all issues with AI agents
In `@web/hooks/useConfetti.ts`:
- Line 24: The local variable interval is typed as any; change it to a correct
timer type to avoid any — replace "const interval: any = setInterval(...)" with
"const interval: ReturnType<typeof setInterval> = setInterval(...)" (or use
NodeJS.Timeout or number depending on runtime) so the variable in useConfetti.ts
and any clearInterval calls have proper typing.
- Around line 15-36: fireFireworks currently omits reduced-motion handling and
never clears its interval; update the confetti calls in fireFireworks to include
disableForReducedMotion: true (alongside the existing defaults object) and
ensure the interval is cleared on unmount by storing the interval id (e.g., in a
ref like fireworksIntervalRef) and calling
clearInterval(fireworksIntervalRef.current) in a useEffect cleanup; keep
randomInRange and defaults as-is, but assign the interval to the ref instead of
a local variable so you can clear it when the component unmounts.
In `@web/package.json`:
- Line 23: Remove the unrelated devDependency entry "@playwright/test":
"^1.58.2" from web/package.json (the dependency line shown in package.json) and
update the lockfile by running your package manager (npm install or yarn
install) so package-lock.json / yarn.lock is updated; commit the package.json
and lockfile changes and open Playwright in a separate PR only when tests/config
are actually added.
|
@jules In
In
|
1 similar comment
|
@jules In
In
|
I have updated |
Acknowledged. I have addressed the feedback regarding |
- Update `useConfetti` to use `useRef` for interval management and proper typing - Add `disableForReducedMotion: true` to fireworks configuration - Ensure fireworks interval is cleared on component unmount - Remove `@playwright/test` from `web/package.json` and update lockfile Co-authored-by: Devasy23 <110348311+Devasy23@users.noreply.github.com>
1 participant
Added confetti and fireworks animations for success states to improve user delight.
The confetti triggers manually when a group is fully settled, and fireworks trigger automatically when recording a payment.
PR created automatically by Jules for task 1525363806836046590 started by @Devasy23
Summary by CodeRabbit
New Features
Documentation